Skip to content

Lucky-number TestDox is more detailed (arrays) [no important files changed]#973

Open
resu-xuniL wants to merge 5 commits into
exercism:mainfrom
resu-xuniL:main
Open

Lucky-number TestDox is more detailed (arrays) [no important files changed]#973
resu-xuniL wants to merge 5 commits into
exercism:mainfrom
resu-xuniL:main

Conversation

@resu-xuniL
Copy link
Copy Markdown
Contributor

With this add, the 'lucky-number' PHPUnit test for arrays is more verbose.
[no important files changed]

@github-actions
Copy link
Copy Markdown

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

@mk-mxp
Copy link
Copy Markdown
Contributor

mk-mxp commented May 25, 2026

Thanks for pointing this out! Instead of fixing this, we'd prefer unrolling the PHPUnit DataProvider. Would you be willing to do that?

Rational in short: DataProviders are a huge burden to students new to programming, as they hide the actual test call behind variables. It is very distracting from learning the type juggling when you do not see the actual literal values passed in.

@resu-xuniL
Copy link
Copy Markdown
Contributor Author

resu-xuniL commented May 25, 2026

Why not, if I have understood the purpose correctly.
Is this supposed to be like this?

/**
 * @task_id 1
 */
#[TestDox('Sums up array [2] and array [7] to 9')]
public function testSumUpBothNumbersSameLength1(): void
{
    $luckynumber = new LuckyNumbers();
    $this->assertEquals(9, $luckynumber->sumUp([2], [7]));
}

/**
 * @task_id 1
 */
#[TestDox('Sums up array [2, 4] and array [5, 7] to 81')]
public function testSumUpBothNumbersSameLength2(): void
 {
    $luckynumber = new LuckyNumbers();
    $this->assertEquals(81, $luckynumber->sumUp([2, 4], [5, 7]));
}

@mk-mxp
Copy link
Copy Markdown
Contributor

mk-mxp commented May 26, 2026

The test methods are OK. The TestDox strings and test names should be more about the intent of the test values (what property is tested by these values?). That is a challenge, I know. It really is good guessing and a bit of fantasy, like:

/** @task_id 1 */
#[TestDox('Sums up two single digit numbers')]
public function testSumsUpTwoSingleDigitNumbers(): void
{
    $luckynumber = new LuckyNumbers();
    $this->assertEquals(9, $luckynumber->sumUp([2], [7]));
}

/** @task_id 1 */
#[TestDox('Sums up two numbers with two digits each')]
public function testSumUpTwoNumbersWithTwoDigitsEach(): void
 {
    $luckynumber = new LuckyNumbers();
    $this->assertEquals(81, $luckynumber->sumUp([2, 4], [5, 7]));
}

Do you accept the challenge?

@resu-xuniL
Copy link
Copy Markdown
Contributor Author

resu-xuniL commented May 26, 2026

I do

@resu-xuniL
Copy link
Copy Markdown
Contributor Author

I am not quite sure about last test.
Hope the others are correct !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants